Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature(esp_tinyusb): Added tusb_teardown() call while tinyusb_driver_uninstall() #39

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

feature(esp_tinyusb): Added tusb_teardown() call while tinyusb_driver_uninstall() #39

wants to merge 13 commits into from

Conversation

roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Jun 11, 2024 •
edited
Loading

Requirements

  • To uninstall tinyusb driver, a teardown tinyusb stack should be called.
  • tud_deinit() feature was implemented in upstream, starting with version v0.17.0.

Description

  • Added tusb_teardown() call while tinyusb_driver_uninstall()
  • Added test app to verify teardown feature

Testing

  1. Install TinyUSB device without any class
  2. Wait SetConfiguration() (tud_mount_cb)
  3. If attempts == 0 goto step 8
  4. Wait TEARDOWN_DEVICE_DETACH_DELAY_MS
  5. Uninstall TinyUSB device
  6. Wait TEARDOWN_DEVICE_INIT_DELAY_MS
  7. Decrease attempts by 1, goto step 3
  8. Wait TEARDOWN_DEVICE_DETACH_DELAY_MS
  9. Uninstall TinyUSB device

Note: when amount of teardown configured to 0, then only one install-uninstall routine is fulfilled.

Related issues

@roma-jam roma-jam self-assigned this Jun 11, 2024
@roma-jam
Copy link
Collaborator Author

roma-jam commented Jun 11, 2024 •
edited
Loading

@tore-espressif , @peter-marcisovsky
This changes unlock the possibility to run usb device tests one by one (espressif/tinyusb#27 should be merged first, though).

I posted two questions for both of you, because it seems that we can make it better and we can use it.

I need that for enumeration driver testing (but maybe I will eliminate esp_tinyusb component from the chain, but anyway), so feel free to check it out when possible.

Meanwhile, I will go and cover the Enum Driver as much as I can.

@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 5 times, most recently from d7cb17d to c14eebb Compare June 11, 2024 12:57
@roma-jam roma-jam marked this pull request as ready for review June 11, 2024 13:14
peter-marcisovsky
peter-marcisovsky reviewed Jun 13, 2024
View reviewed changes
Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some observations from using the test app

device/esp_tinyusb/test_app/sdkconfig.defaults Outdated Show resolved Hide resolved
device/esp_tinyusb/tinyusb.c Show resolved Hide resolved
@finger563
Copy link

Is there a timeline for this feature to be finalized?

@roma-jam
Copy link
Collaborator Author

Hi @finger563 ,

This feature allows the re-configure the driver with these changes: espressif/tinyusb#27
But as far as the same changes are already in the upstream, so we will rebase these changes after updating version to v0.17.0.
For current feature there is no available timeline for now, but the one thing for sure: the update to v0.17.0 will be done first.

Meanwhile, this feature should be available in the upstream of TinyUSB, if anything it is possible to use it.

Sorry for the inconvenience.

@peter-marcisovsky peter-marcisovsky mentioned this pull request Oct 15, 2024
Merged
6 tasks
@roma-jam roma-jam marked this pull request as draft December 4, 2024 14:08
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from 87ddae1 to 85c3227 Compare December 4, 2024 14:25
@roma-jam roma-jam added this to the esp_tinyusb v1.6.0 milestone Dec 5, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from ef29b89 to 7bcc554 Compare December 11, 2024 11:40
This was referenced Dec 12, 2024
Merged
Draft
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 2 times, most recently from da569b5 to 2954367 Compare December 13, 2024 12:09
@roma-jam roma-jam removed the request for review from Dazza0 December 13, 2024 12:20
@roma-jam
Copy link
Collaborator Author

roma-jam commented Dec 13, 2024 •
edited
Loading

@tore-espressif @peter-marcisovsky
This is the enabling of teardown feature via tinyusb_driver_uninstall() call from esp_tinyusb wrapper.

To enable the CI (I am expecting the teardown_device test to fail), it is required to release espressif/tinyusb component with changes from here: espressif/tinyusb#44

Feel free to review the changes in current PR and I will relaunch CI after the tinyusb release (Which I planned on 22th of December, but will do that next Monday, 16th).

UPD: we are running only cdc test, not all of them, so there is not problem with CI

peter-marcisovsky
peter-marcisovsky reviewed Dec 16, 2024
View reviewed changes
Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving some comments.
I am having some issues with running the test locally. Will try to fix it an maybe add something else to the review.

device/esp_tinyusb/test_apps/teardown_device/main/test_teardown.c Outdated Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/main/test_teardown.c Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/pytest_teardown_device.py Outdated Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/main/test_teardown.c Outdated Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/pytest_teardown_device.py Outdated Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/main/test_teardown.c Outdated Show resolved Hide resolved
device/esp_tinyusb/test_apps/teardown_device/pytest_teardown_device.py Outdated Show resolved Hide resolved
@roma-jam roma-jam marked this pull request as draft December 16, 2024 21:21
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 2 times, most recently from 5a9d94d to 4be9e95 Compare December 19, 2024 13:07
@roma-jam roma-jam marked this pull request as ready for review December 19, 2024 13:13
@roma-jam roma-jam marked this pull request as draft December 19, 2024 14:16
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 4 times, most recently from f6dc83c to fae1686 Compare December 19, 2024 22:39
@roma-jam roma-jam mentioned this pull request Dec 20, 2024
Merged
6 tasks
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from dfd0deb to a1e0772 Compare December 20, 2024 12:27
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from a1e0772 to d302232 Compare December 20, 2024 12:28
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from 0ca5094 to 2f2c9de Compare December 20, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peter-marcisovsky peter-marcisovsky peter-marcisovsky left review comments

@greenaddress greenaddress greenaddress left review comments

@tore-espressif tore-espressif Awaiting requested review from tore-espressif

@leeebo leeebo Awaiting requested review from leeebo

At least 1 approving review is required to merge this pull request.

Assignees

@roma-jam roma-jam

Labels
None yet
Projects
None yet
Milestone
esp_tinyusb v1.6.0
Development

Successfully merging this pull request may close these issues.

esp32s3, restarting cdc software serial doesn't work (IDFGH-12813)
6 participants
@roma-jam @finger563 @greenaddress @leeebo @tore-espressif @peter-marcisovsky